Skip to content

Conversation

@MoeMahhouk
Copy link
Contributor

@MoeMahhouk MoeMahhouk commented Jun 16, 2025

Issue Addressed

This pull request introduces workflows and updates to ensure reproducible builds for the Lighthouse project. It adds two GitHub Actions workflows for building and testing reproducible Docker images and binaries, updates the Makefile to streamline reproducible build configurations, and modifies the Dockerfile.reproducible to align with the new build process. Additionally, it removes the reproducible profile from Cargo.toml.

Proposed Changes

New GitHub Actions Workflows:

Build Configuration Updates:

  • Makefile: Refactors reproducible build targets, centralizes environment variables for reproducibility, and updates Docker build arguments for x86_64 and aarch64 architectures.
  • Dockerfile.reproducible: Updates the base Rust image to version 1.86, removes hardcoded reproducibility settings, and delegates build logic to the Makefile.
  • Switch to using jemalloc-sys from Debian repos instead of building it from source. A Debian version is reproducible which is hard to achieve if you build it from source.

Profile Removal:

  • Cargo.toml: Removes the reproducible profile, simplifying build configurations and relying on external tooling for reproducibility.

Additional Info

This is mainly a follow up to this work #6799 where I refine the reproducible build configuration to simplify the CI workflow to generate the reproducible images and pushes them to DockerHub. I also added a cron job workflow (inspired from the Reth repo) that checks every two days or pull requests that touches files that might affect reproducibility to catch potential regressions.
In case, this is too much, let me know and I can create a separate PR for this to be merged later when necessary

close #7486
close #7485

@cla-assistant
Copy link

cla-assistant bot commented Jun 16, 2025

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Jun 16, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ MoeMahhouk
❌ Ubuntu


Ubuntu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@MoeMahhouk MoeMahhouk force-pushed the unstable branch 2 times, most recently from 82dcfef to 238fbaa Compare June 17, 2025 10:39
@chong-he
Copy link
Member

Doing some testing on this, will post the comment when ready

@mergify
Copy link

mergify bot commented Jun 23, 2025

Some required checks have failed. Could you please take a look @MoeMahhouk? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 23, 2025
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I left some comments after doing some testing

@MoeMahhouk MoeMahhouk marked this pull request as draft June 24, 2025 15:56
@MoeMahhouk
Copy link
Contributor Author

Thanks for the PR. I left some comments after doing some testing

Thanks for reviewing it.
I will do some rework and refinements to this PR to fix the issues and address your comments.

@MoeMahhouk MoeMahhouk requested a review from chong-he June 30, 2025 15:55
@chong-he
Copy link
Member

chong-he commented Jul 1, 2025

Thanks for the PR. I left some comments after doing some testing

Thanks for reviewing it. I will do some rework and refinements to this PR to fix the issues and address your comments.

Is there a reason why is this PR still a draft?

@MoeMahhouk
Copy link
Contributor Author

Thanks for the PR. I left some comments after doing some testing

Thanks for reviewing it. I will do some rework and refinements to this PR to fix the issues and address your comments.

Is there a reason why is this PR still a draft?

Not really, I am waiting for your final feedback to open it for review/merge.
I will open it now for review and if you find anything needed for iteration, we can switch it back again.

@MoeMahhouk MoeMahhouk marked this pull request as ready for review July 1, 2025 15:30
@chong-he chong-he added the test-reproducible for testing reproducible builds label Jul 2, 2025
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at this a second time and try to suggest some simplifications so that it is easier to maintain and make the code cleaner.

Also, I would prefer to not have the symbol (green tick, cross etc) to keep the code clean.

@chong-he chong-he removed the stale Stale PRs that have been inactive and is now outdated label Nov 4, 2025
@MoeMahhouk MoeMahhouk requested a review from chong-he November 5, 2025 16:05
@MoeMahhouk MoeMahhouk force-pushed the unstable branch 3 times, most recently from 2a7e1a6 to 1e17908 Compare November 6, 2025 19:22
@MoeMahhouk
Copy link
Contributor Author

MoeMahhouk commented Nov 6, 2025

I believe I addressed all your feedback/comments and also added reproducibility fix for jemalloc-sys which is used to be not reproducible on different host machines.
I kindly ask your review again on the changes as well as the responses to the comments if you are ok with the current state
@chong-he @michaelsproul

@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Nov 7, 2025
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment here. I am unsure about other changes, will need another review from a dev

Can we also remove the echo that are not strictly necessary? For example,

echo "Published: ${IMAGE_NAME}:${VERSION}"

@MoeMahhouk
Copy link
Contributor Author

Just a small comment here. I am unsure about other changes, will need another review from a dev

Can we also remove the echo that are not strictly necessary? For example,

echo "Published: ${IMAGE_NAME}:${VERSION}"

I removed the majority of the "echo" statements. If you think any other one is not necessary, let me know and I'll adjust accordingly

@chong-he
Copy link
Member

chong-he commented Nov 11, 2025

Just a small comment here. I am unsure about other changes, will need another review from a dev
Can we also remove the echo that are not strictly necessary? For example,
echo "Published: ${IMAGE_NAME}:${VERSION}"

I removed the majority of the "echo" statements. If you think any other one is not necessary, let me know and I'll adjust accordingly

Thanks.

We need a review from @michaelsproul when he has time. I am not all that familiar with docker and stuff. I think this PR is now clean enough for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra-ci ready-for-review The code is ready for review test-reproducible for testing reproducible builds

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants